Skip to content

Refactor (packages/tui/src/util/error.ts): Function with many returns (count = 13): cliErrorMessage - #65

Open
Ravin-Kumar wants to merge 4 commits into
CMU-17313Q:mainfrom
Ravin-Kumar:refactor-tui-util-errors
Open

Refactor (packages/tui/src/util/error.ts): Function with many returns (count = 13): cliErrorMessage#65
Ravin-Kumar wants to merge 4 commits into
CMU-17313Q:mainfrom
Ravin-Kumar:refactor-tui-util-errors

Conversation

@Ravin-Kumar

@Ravin-Kumar Ravin-Kumar commented Sep 4, 2026

Copy link
Copy Markdown

P1B: Starter Task: Refactoring PR

1. Issue

Link to the associated GitHub issue:
This PR fixes issue #58.

Full path to the refactored file:
packages/tui/src/util/error.ts

What do you think this file does?
This file is used to format generic, unformatted errors into display-ready errors.

What is the scope of your refactoring within that file?
This PR affects the cliErrorMessage function within error.ts.

Which Qlty‑reported issue did you address?
packages/tui/src/util/error.ts:5 Function with many returns (count = 13): cliErrorMessage

2. Refactoring

How did the specific issue you chose impact the codebase’s maintainability?

The multiple return statements made it confusing to understand the control flow of the application, especially when multiple return statements seemingly returned the same value under different conditionals.

What changes did you make to resolve the issue?

I changed the function by consolidating similar return values and making control flow simpler to understand by using conditionals to explicitly separate clauses with if/else if blocks. The return value is now set in a variable and only returns at the end of the function.

How do your changes improve maintainability? Did you consider alternatives?

As the return value is now stored in a variable before being returned, manipulation of the value is much simpler. Rather than forcing multiple steps onto one line, more readable formatting can be done by manipulating the variable until the error message is fully formatted.

3. Validation

How did you validate that the change is correct?

Since none of the test cases covered the function cliErrorMessage, tests were created to validate that function continued to work correctly. The tests were run on both the previous and refactored version of the function to ensure that the function's behavior stayed as initially intended. The file packages/tui/test/util/error.test.ts contains the new test cases.

Attach a screenshot of the test coverage showing the lines were executed by the tests.

test-coverage

Attach a screenshot showing the tests that cover the change passing during CI

This shows the new tests passing on the previous version of the function:
bun-test-before

This shows the tests passing on the refactored version of the function:
bun-test-after

Attach a screenshot of qlty smells --no-snippets <full/path/to/file.ts> showing fewer reported issues after the changes.

Here is the qlty smells before and after the refactor:
desmellified

Screenshot of bun lint passing locally
bun-lint

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant